Skip to content

????? add slice::array_chunks #72334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 4 commits into from
Closed

????? add slice::array_chunks #72334

wants to merge 4 commits into from

Conversation

lcnr
Copy link
Contributor

@lcnr lcnr commented May 18, 2020 •
edited
Loading

adds a currently somewhat unuseable implementation of #60735.
cc @DutchGhost

I am not sure if this does more harm than good by showing it in the stable docs, but this felt like a nice challenge of my knowledge of the current state of const generics.

r? @varkor for the const generics bit

edit: the biggest problem is #71154, which prevents all uses of slice.array_chunks::<{expr}>() and some type inference stuff which may already be fixed on the currently nightly

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 18, 2020
@DutchGhost
Copy link
Contributor

DutchGhost commented May 18, 2020 •
edited
Loading

Oh! array_chunks, I like that name!

I remember the compiler got a little wonky when array_chunks was a method on [T].

I also remember it was a little less wonky when using a freestanding function as I wrote in #60735 (comment).

Perhaps it's something to investigate if we want this feature to (finally) be usable in nightly?

However I found the free standing function a bit of a downside if it goes for how the resulting code looks at userlevel. <[...]>::array_chunks(slice) vs slice.array_chunks().

@lcnr
Copy link
Contributor Author

lcnr commented May 18, 2020 •
edited
Loading

To be fair, the compiler is always a little wonky atm 😆

Free standing functions work as const_chunks_exact::<_, 3> is not a type dependent path,
i.e. we already know which method is used here during ast lowering.

For v.array_chunks, we must know the type of v to get the method definition of array_chunks, and therefore the expected type of 3. This would currently cause a cycle error if we tried to look it up. I tried to explain this problem in more detail in the description of #71154.

<[...]>::array_chunks(slice) also works for methods taking self

@DutchGhost
Copy link
Contributor

Can we make it a method, but enforce function style calling for now? Mainly I find it concerning that when called as method, rustc ICE's. I don't think its good for libcore to have code that crashes rustc when called 'wrong'

@leonardo-m
Copy link

Panics if N is 0.

Wish this to happen at compile-time.

@lcnr
Copy link
Contributor Author

lcnr commented May 19, 2020 •
edited
Loading

Can we make it a method, but enforce function style calling for now?

Sadly not, as slice.array_chunks::<{expr}>() cases an ice even in cases where slice.array_chunks is invalid (e.g. array chunks is not a method of slice).

So even if we change array_chunks to take slice: &Self instead, we still get the same ICE and also can't use let [a, b] = slice.array_chunks().next().unwrap(); anymore.

@varkor varkor marked this pull request as draft May 24, 2020 16:53
@varkor
Copy link
Member

varkor commented May 24, 2020

I'm going to mark this as a draft for now, because I don't think we're ready to add this even unstably yet, when it's too prone to ICEing. Hopefully we'll be able to get the remaining issues ironed out before too long.

@Muirrum
Copy link
Member

Muirrum commented Jun 20, 2020

@rustbot modify labels to S-waiting-on-author -S-waiting-on-review

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 20, 2020
@Dylan-DPC-zz
Copy link

I'm going to close this. Leaving it open will just accumulate conflicts and things may change in the future. Better to create a new PR when revisiting it.

@Dylan-DPC-zz Dylan-DPC-zz added S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 20, 2020
@lcnr lcnr deleted the array_chunks branch June 20, 2020 16:31
@lcnr lcnr restored the array_chunks branch July 9, 2020 13:25
@lcnr lcnr mentioned this pull request Jul 15, 2020
Merged
@lcnr
Copy link
Contributor Author

lcnr commented Jul 15, 2020

Reopened as #74373.

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 1, 2020
@bors
Auto merge of rust-lang#74373 - lcnr:array_chunks, r=withoutboats
b5eae9c 
add `slice::array_chunks` to std

Now that rust-lang#74113 has landed, these methods are suddenly usable. A rebirth of rust-lang#72334

Tests are directly copied from `chunks_exact` and some additional tests for type inference.

r? @withoutboats as you are both part of t-libs and working on const generics. closes rust-lang#60735
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@varkor varkor

Labels
S-inactive Status: Inactive and waiting on the author. This is often applied to closed PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@lcnr @DutchGhost @leonardo-m @varkor @Muirrum @Dylan-DPC-zz @rust-highfive @rustbot